fix: Eliminate port allocation race condition in all affected test files #1528
+388
−50
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Fixes a TOCTOU (Time-of-Check-Time-of-Use) race condition where multiple pytest-xdist workers would attempt to bind to the same port, causing tests to fail with "address already in use" errors or connect to the wrong server. ## Root Cause The server_port() fixture would: 1. Bind to port 0 to get a free port 2. Immediately close the socket, freeing the port 3. Return the port number When tests ran in parallel with pytest-xdist, multiple workers could get the same "free" port between steps 2 and 3, leading to conflicts. ## Solution Implement worker-specific port ranges using pytest-xdist's worker_id: - Each worker gets a dedicated range from 40000-49999 - Port ranges are calculated based on PYTEST_XDIST_WORKER_COUNT - Guarantees no overlap between workers ## Changes - Add parse_worker_index() to extract worker number from worker_id - Add calculate_port_range() to compute non-overlapping port ranges - Refactor get_worker_specific_port() to use the pure functions above - Update server_port fixture to use worker-specific ports - Add comprehensive unit tests (28 tests) covering all edge cases Tests now pass reliably with 14 workers in parallel.
Extended the fix from test_integration.py to all test files that suffered
from the same TOCTOU (Time-of-Check-Time-of-Use) race condition. When
running with pytest-xdist parallel workers, multiple workers would get
the same "free" port from `socket.bind(("127.0.0.1", 0))`, leading to:
- "address already in use" errors
- Tests connecting to wrong servers
Solution:
- Updated 7 test files to use `get_worker_specific_port(worker_id)`:
* tests/server/test_streamable_http_security.py
* tests/server/test_sse_security.py
* tests/shared/test_streamable_http.py (3 fixtures)
* tests/shared/test_sse.py
* tests/shared/test_ws.py
* tests/client/test_http_unicode.py
* tests/client/test_notification_response.py
- Fixed socket cleanup issue in tests/test_test_helpers.py
Each worker now gets an exclusive port range from 40000-59999, eliminating
the race condition across all test files that start servers.
Testing:
- All 703 tests pass with pytest-xdist parallel execution
- Port allocation now consistent across all parallel workers
maxisbey
added
bug
|
actually, I don't like this approach anymore. It's a bit late at night so I'm going to leave it as draft for now, but some other options I was thinking are:
Although none of them really are clean IMO. So going to leave this for now |
felixweinberger
added
the
pending publish
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR extends the port allocation race condition fix to all test files affected by the same TOCTOU issue, eliminating intermittent test failures when running with pytest-xdist parallel workers.
Motivation and Context
When running tests with pytest-xdist parallel workers, multiple workers would receive the same "free" port from
socket.bind(("127.0.0.1", 0)), causing a TOCTOU (Time-of-Check-Time-of-Use) race condition. This manifested as:This builds on the initial fix in tests/server/fastmcp/test_integration.py by applying the same solution to 7 additional test files that had the identical issue.
How Has This Been Tested?
PYTEST_DISABLE_PLUGIN_AUTOLOAD="" uv run --frozen pytest tests/ -vEach pytest-xdist worker now receives an exclusive port range from 40000-59999 based on its worker ID, eliminating port conflicts entirely.
Breaking Changes
None. This is an internal test infrastructure change with no impact on the public API or user-facing functionality.
Types of changes
Checklist
Additional context
Files modified:
tests/server/test_streamable_http_security.py- Updatedserver_portfixturetests/server/test_sse_security.py- Updatedserver_portfixturetests/shared/test_streamable_http.py- Updated 3 port fixtures (basic, json, event server)tests/shared/test_sse.py- Updatedserver_portfixturetests/shared/test_ws.py- Updatedserver_portfixturetests/client/test_http_unicode.py- Updatedunicode_server_portfixturetests/client/test_notification_response.py- Updatednon_sdk_server_portfixturetests/test_test_helpers.py- Fixed socket cleanup to prevent ResourceWarningImplementation details:
All affected fixtures now use
get_worker_specific_port(worker_id)instead of the race-pronesocket.bind(("127.0.0.1", 0))pattern. The helper function allocates worker-specific port ranges ensuring no overlap between parallel test workers.